-
Notifications
You must be signed in to change notification settings - Fork 305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pull: Lower max concurrent HTTP requests to 2 #3065
Conversation
I believe this may help re-enabling the libcurl low-speed limit checks in the case where we have a full 8 HTTP requests outstanding with the remote server, but it's actually throttling us. In practice, we really don't need more than two. Experimenting with this using the builtin Go HTTP server, this actually seems to increase performance; but I get pretty wild timing variations overall.
Ah yes of relevance here, this looks like the librepo equivalent: https://github.com/rpm-software-management/librepo/blob/27cb3114ccce72814df305aa7342ee5bc88e3ac0/librepo/handle.h#L72 |
This looks good to me, we can also just make it configurable in the future if for some reason this becomes an issue. |
This seems like a pretty significant change. We've been operating the soup backend with 8 concurrent requests for years. Do you have any data to back up this change? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's have the above discussion before merging.
One thing to bear in mind here is that in practice the important thing is to keep the "pipeline" full - and we're still going to be doing that in general.
As I said in the commit message when using locally, it overall improves performance - although this is with libcurl. What I think would be really helpful is to set up a CI/test flow that better reproduces some "real world" conditions without actually hitting up the internet.
Yes agree, we should make this configurable. |
Can you explain that? What do "in practice" and "in general" mean in this context? |
We'll make this part of #2843 |
I believe this may help re-enabling the libcurl low-speed limit checks in the case where we have a full 8 HTTP requests outstanding with the remote server, but it's actually throttling us.
In practice, we really don't need more than two. Experimenting with this using the builtin Go HTTP server, this actually seems to increase performance; but I get pretty wild timing variations overall.